fix: issue #112 #262
fix: issue #112 #262
Conversation
|
@maxceem please review |
src/hoc/withAuthentication/index.js
Outdated
| @@ -96,8 +105,14 @@ export default function withAuthentication(Component) { | |||
| <LoadingIndicator error={authError || teamMembersLoadingError} /> | |||
| ))} | |||
|
|
|||
| {/* Show component only if v5 user profile load error */} | |||
| {isLoggedIn === true && v5UserProfileLoadingError && ( | |||
| <LoadingIndicator error={"Error: Network Error"} /> | |||
There was a problem hiding this comment.
@yoution sorry, I didn't mean to show error Network Error I mean to show error which happens during loading user profile. We have to pass it there.
There was a problem hiding this comment.
I don't know how to repreduce the error case
There was a problem hiding this comment.
you may change endpoint to incorrect one, like below, it should cause some error during request, and it should be shown on the page
return axios.get(`${config.API.V5}/taas-teams/meBROKEN`);
There was a problem hiding this comment.
yeah, looks good, this is what was needed
|
@yoution also, could we please name action as |
There was a problem hiding this comment.
@yoution it works almost perfect for me locally.
But there is a small improvement to make.
When we wait for v5 user profile loading, the loading indicator has to be shown
It's hard to see, but when V5 user profile is being loaded, the loading indicator is not shown, see how it looks if I slow down loading of v5 user profile to 10 seconds https://monosnap.com/file/CuX6Ahly89FgAdpn1QJFRgh7OcJRF9
How to reproduce it. In file src/services/teams.js
- add import
import { delay } from "utils/helpers";on line 6 - update code of
getUserProfiletoexport const getUserProfile = () => { return delay(10000).then(() => axios.get(`${config.API.V5}/taas-teams/me`)); };
So you could see how it would look like if loading of V5 user profile is slow.
It would be nice (but not required) if existent code could be reused, we already have 2 places in code: 1 place to show loading indicator or error, and another one to show component if all is loaded:
One more thing.
Could we please also rename
- action method
authLoadUserProfiletoauthLoadV5UserProfile - and service method
getUserProfiletogetV5UserProfile
to make it clear that this is for V5 user profile to avoid confusion in the future.
No description provided.